Upgrade to Rails 8.1 by jazairi · Pull Request #1515 · MITLibraries/thing

jazairi · 2026-07-21T00:09:16Z

This was completed in separate commits to make individual changes easier to parse. Notable changes that affect ETD:

Positional enums are now required
Pluralization requires more explicit instructions for custom and irregular verbs
Devise config loads before the environment file, requiring fallbacks for config variables

In order to avoid further convoluting the PR, I've avoided addressing two issues:

Deprecation warnings for ActiveSupport::Multibyte::Chars and String#mb_chars. These should be addressed as part of the Rails 8.2 upgrade.
Reintroduction of annotate via the annotaterb gem. Running annotations touched many files. This change will occur in a separate PR.

Developer

All new ENV is documented in README
All new ENV has been added to Heroku Pipeline, Staging and Prod
ANDI or Wave has been run in accordance to
our guide and
all issues introduced by these changes have been resolved or opened as new
issues (link to those issues in the Pull Request details above)
Stakeholder approval has been confirmed (or is not needed)

Code Reviewer

The commit message is clear and follows our guidelines
(not just this pull request message)
There are appropriate tests covering any new functionality
The documentation has been updated or is unnecessary
The changes have been verified
New dependencies are appropriate or there were no changes

Requires database migrations?

NO

Includes new or updated dependencies?

YES

This was noted as a depreciation warning when running tests

Pin theme gem to v1.5

Rails 8 initializes Devise before loading test env

Rails 8 requires explicit pluralization rules for irregular verbs

accesslint

Found 4 issues across 1 rule.

accesslint · 2026-07-21T00:09:34Z

@@ -0,0 +1,135 @@
+<!doctype html>
+
+<html lang="en">


Best Practice: Page does not contain a level-one heading.

Page should contain a level-one heading.

Details

Page title: "The server cannot process the request due to a client error (400 Bad Request)", Main content: "The server cannot process the request due to a client error. Please check the request and try again."

A level-one heading (<h1> or role='heading' with aria-level='1') helps users understand the page topic and provides a landmark for screen reader navigation. Each page should have at least one level-one heading that describes the main content, typically matching or similar to the page title.

accesslint · 2026-07-21T00:09:34Z

-</body>
+<!doctype html>
+
+<html lang="en">


Best Practice: Page does not contain a level-one heading.

Page should contain a level-one heading.

Details

Page title: "The page you were looking for doesn't exist (404 Not found)", Main content: "The page you were looking for doesn't exist. You may have mistyped the address or the page may have "

A level-one heading (<h1> or role='heading' with aria-level='1') helps users understand the page topic and provides a landmark for screen reader navigation. Each page should have at least one level-one heading that describes the main content, typically matching or similar to the page title.

accesslint · 2026-07-21T00:09:34Z

-</body>
+<!doctype html>
+
+<html lang="en">


Best Practice: Page does not contain a level-one heading.

Page should contain a level-one heading.

Details

Page title: "Your browser is not supported (406 Not Acceptable)", Main content: "Your browser is not supported. Please upgrade your browser to continue."

A level-one heading (<h1> or role='heading' with aria-level='1') helps users understand the page topic and provides a landmark for screen reader navigation. Each page should have at least one level-one heading that describes the main content, typically matching or similar to the page title.

accesslint · 2026-07-21T00:09:34Z

-</body>
+<!doctype html>
+
+<html lang="en">


Best Practice: Page does not contain a level-one heading.

Page should contain a level-one heading.

Details

Page title: "The change you wanted was rejected (422 Unprocessable Entity)", Main content: "The change you wanted was rejected. Maybe you tried to change something you didn't have access to. I"

A level-one heading (<h1> or role='heading' with aria-level='1') helps users understand the page topic and provides a landmark for screen reader navigation. Each page should have at least one level-one heading that describes the main content, typically matching or similar to the page title.

coveralls · 2026-07-21T00:11:52Z

coverage: 98.25%. remained the same — rails8 into main

JPrevost

Very minor suggested changes:

Reintroduce:

config.public_file_server.enabled = ENV['RAILS_SERVE_STATIC_FILES'].present?
production log format stanza

Comment out line for a check we don't need in

bin/ci

JPrevost · 2026-07-21T12:22:19Z

  </title>
  <%= render "stylesheet" %>
-  <%= stylesheet_link_tag "administrate", data: { turbo_track: "reload" } %>
+  <%= stylesheet_link_tag "administrate/application", data: { turbo_track: "reload" } %>


Interesting. So this is how we were able to stop adding this to the precompiled I guess a couple commits back?

Yeah, propshaft (the Rails 8 pipeline) uses logical paths instead of precompiled.

JPrevost · 2026-07-21T12:28:21Z

-  # config.require_master_key = true

-  # Disable serving static files from `public/`, relying on NGINX/Apache to do so instead.
-  config.public_file_server.enabled = ENV['RAILS_SERVE_STATIC_FILES'].present?


Do we not still need this? I suppose if the default is to serve we don't. I'm not sure how to test in Heroku that the static files from the public directory are being served in this PR build.

We definitely set RAILS_SERVE_STATIC_FILES so I'm reluctant to remove this unless you are 100% confident the new default is to serve them since we don't run behind nginx/httpd so rails has to serve them.

I don't think we do, but I agree that it's wise to keep it just in case. I'm also not sure how to confirm in the PR build.

JPrevost · 2026-07-21T12:31:29Z

-  # Do not fall back to assets pipeline if a precompiled asset is missed.
-  config.assets.compile = false
+  # Cache assets for far-future expiry since they are all digest stamped.
+  config.public_file_server.headers = { "cache-control" => "public, max-age=#{1.year.to_i}" }


I don't understand how this wasn't set before. 🤷🏻

JPrevost · 2026-07-21T12:42:35Z

-  # Log to STDOUT by default
-  config.logger = ActiveSupport::Logger.new(STDOUT)
-    .tap  { |logger| logger.formatter = ::Logger::Formatter.new }
-    .then { |logger| ActiveSupport::TaggedLogging.new(logger) }


I think we want to keep this logging config and remove the new logger statement on line 38. Without it we lose the log level . We can tune a bit more later as we end up sort of double logging timestamps now that I've looked closer at this but for now let's revert this and work towards standardizing our rails apps logging config in the future.

JPrevost · 2026-07-21T12:42:59Z

-# application.js, application.css, and all non-JS/CSS in the app/assets
-# folder are already added.
-# Rails.application.config.assets.precompile += %w( admin.js admin.css )
-Rails.application.config.assets.precompile += %w( administrate.css )


This confused me, but it seems to work so

See my comment above about propshaft.

JPrevost · 2026-07-21T12:46:12Z

+
+  step "Style: Ruby", "bin/rubocop"
+
+  step "Security: Importmap vulnerability audit", "bin/importmap audit"


Since we aren't using import maps, we can comment out this line. We probably also aren't immediately adopting running bin/ci but it probably makes sense to comment this out anyone just in case someone runs it. I commented it out in the timdex upgrade so if nothing else this will be consistent :)

Will do. I missed that you'd commented it out in the TIMDEX upgrade.

jazairi added 12 commits July 16, 2026 12:34

Use positional enums to prepare for Rails 8

07bb8d9

This was noted as a depreciation warning when running tests

Bundle update to Rails 8.0 and load 8.0 defaults

c124f13

Pin theme gem to v1.5

Run rails app:update generated files

0dd43a1

Add rails health check

72b8be6

Remove 8.0 defaults initializer

af94d11

Update renovate.json to support Rails 8

2ddc9fb

Bundle update to Rails 8.1 and load 8.1 defaults

171dfc2

rails app:update to 8.1

eb212f5

Remove 8.1 new framework defaults initializer

ecf188b

Map stylesheet links to manifest declaration

29574ad

Add fallback for unset env in Devise config

f097d22

Rails 8 initializes Devise before loading test env

Fix pluralization

8a93197

Rails 8 requires explicit pluralization rules for irregular verbs

accesslint Bot reviewed Jul 21, 2026

View reviewed changes

mitlib temporarily deployed to thesis-submit-pr-1515 July 21, 2026 00:10 Inactive

jazairi requested review from JPrevost and matt-bernhardt July 21, 2026 00:56

JPrevost self-assigned this Jul 21, 2026

JPrevost reviewed Jul 21, 2026

View reviewed changes

Implement code review feedback

c5b90cf

jazairi temporarily deployed to thesis-submit-pr-1515 July 21, 2026 16:34 Inactive

jazairi requested a review from JPrevost July 21, 2026 16:36

JPrevost approved these changes Jul 21, 2026

View reviewed changes

jazairi merged commit 7ad4b48 into main Jul 21, 2026
2 checks passed

jazairi deleted the rails8 branch July 21, 2026 17:27


		step "Style: Ruby", "bin/rubocop"

		step "Security: Importmap vulnerability audit", "bin/importmap audit"

Uh oh!

Conversation

jazairi commented Jul 21, 2026

Developer

Code Reviewer

Requires database migrations?

Includes new or updated dependencies?

Uh oh!

accesslint Bot left a comment

Choose a reason for hiding this comment

Uh oh!

accesslint Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

accesslint Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

accesslint Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

accesslint Bot Jul 21, 2026

Choose a reason for hiding this comment

Uh oh!

coveralls commented Jul 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JPrevost left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Jul 21, 2026 •

edited

Loading